chore: remove certificate validator#1730
Conversation
gsanchezgavier
force-pushed
the
gsanchez/chore/remove-cert-sign-impl
branch
from
ddb9042 to
4ceb3a6
Compare
gsanchezgavier
force-pushed
the
gsanchez/chore/remove-cert-sign-impl
branch
2 times, most recently
from
c6975fd to
19ae8fa
Compare
sigilioso
left a comment
sigilioso
left a comment
There was a problem hiding this comment.
🧹 🧹 🧹
Thanks for taking care of this! If left some relatively unrelated questions and you'll need a rebase, but LGTM 🚀
| let Some(public_key_server_url) = config.public_key_server_url else { | ||
| return Err(SignatureValidatorError::BuildingValidator( | ||
| "missing public_key_server_url configuration".to_string(), | ||
| )); | ||
| }; |
There was a problem hiding this comment.
We fail if enabled and url is not set, nice!
There was a problem hiding this comment.
yes , it shouldn't happen on any existing flow becase recipe and helm chart sets the corresponding url but just in case
| SignatureValidator::new(fleet_config.signature_validation, config.proxy) | ||
| }) | ||
| .transpose()? | ||
| .unwrap_or(SignatureValidator::Noop); | ||
| .unwrap_or(SignatureValidator::new_noop()); |
There was a problem hiding this comment.
We also need the the Noop thing here because we need to build a signature validator even if OpAMP is disabled, right? 🙃
Someday we will take care of the whole let Some(opamp) else thing...
There was a problem hiding this comment.
yes and in this case the warn message doesn't make sense, it only appears whenver opamp is enabled and sig verificatino is disabled
gsanchezgavier
force-pushed
the
gsanchez/chore/remove-cert-sign-impl
branch
from
19ae8fa to
f893c04
Compare
2 participants
NOTE: To be merged after FC deploys the new signature.